fix(mcp): allow create_record for views and added tests to verify the…#3217
fix(mcp): allow create_record for views and added tests to verify the…#3217anushakolan wants to merge 1 commit intorelease/1.7from
Conversation
… same. (#3196) Closes #3194 - MCP `create_record` tool incorrectly blocks views with error `"The create_record tool is only available for tables."` 1. Views are the required workaround for unsupported SQL data types (e.g., vector columns). Users could create views that omit unsupported columns and perform DML operations against those views. 2. This bug prevents INSERT operations via MCP on view entities, breaking a critical workflow for vector database scenarios. 1. Modified `CreateRecordTool.cs` to allow both tables and views to pass source type validation 2. Changed `else` block (which caught views) to`else if (EntitySourceType.StoredProcedure)` so only stored procedures are blocked 3. Views now fall through to the mutation engine, which already supports INSERT on updateable views 4. Updated error message to `"The create_record tool is only available for tables and views."` 5. Added 8 unit tests validating all DML tools (CreateRecord, ReadRecords, UpdateRecord, DeleteRecord) work with both Table and View source types - [ ] Integration Tests - [X] Unit Tests 1. `DmlTool_AllowsTablesAndViews` - 8 DataRow test cases verifying no `InvalidCreateTarget` error for views 2. Existing REST integration tests `InsertOneInViewTest` already validate view INSERT via same mutation engine Manually tested via MCP Inspector, to verify `create_record` calls succeeds on a view. --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Aniruddh Munde <anmunde@microsoft.com> (cherry picked from commit b5841fd)
anushakolan
requested review from
Alekhya-Polavarapu,
Aniruddh25,
RubenCerna2079,
aaronburtle,
neeraj-sharma2592,
rusamant,
sourabh1007,
souvikghosh04,
stuartpa and
vadeveka
as code owners
|
/azp run |
|
Azure Pipelines successfully started running 6 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
This backport updates MCP DML tools to treat view-backed entities as valid targets (not just tables), restoring create_record behavior for scenarios where views are used as a workaround for unsupported SQL types.
Changes:
- Allow
create_recordto proceed for view entities by removing the view-blocking path and skipping request-body validation for views. - Add explicit “table-or-view only” validation to
read_recordsandupdate_record. - Add a new test suite intended to validate entity-level tool enablement and table/view support.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| src/Azure.DataApiBuilder.Mcp/BuiltInTools/CreateRecordTool.cs | Allows view entities and skips request validation for views; adds explicit source-type validation. |
| src/Azure.DataApiBuilder.Mcp/BuiltInTools/ReadRecordsTool.cs | Adds explicit table/view source-type validation after metadata resolution. |
| src/Azure.DataApiBuilder.Mcp/BuiltInTools/UpdateRecordTool.cs | Adds explicit table/view source-type validation after metadata resolution. |
| src/Service.Tests/Mcp/EntityLevelDmlToolConfigurationTests.cs | New tests for entity-level enablement + view support (currently has correctness and dependency-mocking issues). |
| // Validate it's a table or view - stored procedures use execute_entity | ||
| if (dbObject.SourceType != EntitySourceType.Table && dbObject.SourceType != EntitySourceType.View) | ||
| { | ||
| return McpResponseBuilder.BuildErrorResult(toolName, "InvalidEntity", $"Entity '{entityName}' is not a table or view. For stored procedures, use the execute_entity tool instead.", logger); | ||
| } |
There was a problem hiding this comment.
CreateRecordTool performs source-type validation after role-context + permission checks. For non-table/view entities (e.g., stored procedures) this can return PermissionDenied instead of the intended "InvalidEntity" guidance, and it’s inconsistent with Read/Update/Delete which validate source type immediately after metadata resolution. Move the table/view validation to right after TryResolveMetadata so the tool reliably returns the correct error and short-circuits earlier.
| // Assert - Should NOT be a source type blocking error (InvalidEntity) | ||
| // Other errors like missing metadata are acceptable since we're testing source type validation | ||
| if (result.IsError == true) | ||
| { | ||
| JsonElement content = ParseResultContent(result); | ||
|
|
||
| if (content.TryGetProperty("error", out JsonElement error) && | ||
| error.TryGetProperty("type", out JsonElement errorType)) | ||
| { | ||
| string errorTypeValue = errorType.GetString() ?? string.Empty; | ||
|
|
||
| // This error type indicates the tool is blocking based on source type | ||
| Assert.AreNotEqual("InvalidEntity", errorTypeValue, | ||
| $"{sourceType} entities should not be blocked with InvalidEntity"); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
DmlTool_AllowsTablesAndViews only asserts the error type is not "InvalidEntity". This won’t catch the original create_record regression (which returned a different error code, e.g., InvalidCreateTarget), and CreateRecordTool’s source-type check happens after authorization so it may never be exercised with the current IAuthorizationResolver mock setup. Tighten this test to assert the specific source-type blocking error(s) are not returned (including the previous create_record code) and configure authorization mocks so the tool can reach the source-type validation path.
| private static async Task<JsonElement> RunToolAsync(IMcpTool tool, JsonDocument arguments, IServiceProvider serviceProvider) | ||
| { | ||
| CallToolResult result = await tool.ExecuteAsync(arguments, serviceProvider, CancellationToken.None); | ||
| return ParseResultContent(result); | ||
| } |
There was a problem hiding this comment.
RunToolAsync re-executes the tool even when the caller already has a CallToolResult (e.g., DmlTool_RespectsEntityLevelDmlToolDisabled executes twice). That can introduce side-effects and makes debugging harder. Prefer parsing the JSON from the original CallToolResult (similar to ParseResultContent) instead of invoking ExecuteAsync again.
Why make this change?
This backport fixes MCP behavior where create_record incorrectly rejected view-backed entities with the error that the tool is only available for tables.
Views are a required workaround for unsupported SQL data types in some scenarios (e.g., vector columns), so this fix restores expected INSERT behavior for those entities.
Note: This backport includes prerequisite PRs #2989 and #3084 which were not previously in release/1.7 but are required for the test infrastructure.
What is this change?
Cherry-picked PR:
How was this tested?
create_recordagainst view-backed entities.Sample Request(s)
N/A